Skip to content

Conversation

@samgoodgame
Copy link
Contributor

Description

Enhancements to the personalized learning sample:

  • Demo video — Added YouTube walkthrough showing the sample in action
  • Server-side auth — Consolidated access control to API server (removed client-side enforcement)
  • CSS fixes — Fixed A2UI container overflow so flashcards/quizzes don't extend past message bounds
  • Design notes — Documented intent-based routing pattern and CORS considerations

Pre-launch Checklist

  • I signed the CLA.
  • I read the Contributors Guide.
  • I read the Style Guide.
  • I have added updates to the CHANGELOG. (N/A -- this is just a sample)
  • I updated/added relevant documentation.
  • My code changes (if any) have tests. (N/A -- this is just a sample, no test framework)

If you need help, consider asking for advice on the discussion board.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant and valuable improvements to the personalized learning sample. The move to server-side authentication is a major step forward for security and clarity, and the refactoring of the agent code to remove legacy wrappers simplifies the architecture. The updated README is much more concise and user-friendly. I've found a critical security issue related to disabling SSL verification and a couple of other points regarding a hardcoded project ID and the removal of tests. Overall, great work on enhancing the sample!

samgoodgame and others added 2 commits January 28, 2026 21:35
Addresses Gemini code review feedback:
- Replace disabled SSL verification (CERT_NONE) with certifi CA bundle
  for secure GitHub content fetches
- Remove hardcoded project ID from .firebaserc (use placeholder)
- Remove unused dependencies (uvicorn, fastapi, pydantic)
- Delete unused agent/Dockerfile
@zeroasterisk zeroasterisk self-assigned this Jan 29, 2026
@zeroasterisk zeroasterisk self-requested a review January 29, 2026 05:24

# Model configuration
LITELLM_MODEL=gemini-2.5-flash
GENAI_MODEL=gemini-2.5-flash
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually used?

I see MODEL_ID in deleted code, but I'm just asking for a 2x check that all the configuration variables line up for you. Can you delete you .env clear your existing auths and run this from this template?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent directory .env.template is used, but this one at agent/.env.template is redundant -- I committed a change removing this file.

GENAI_MODEL is referenced in four places (agent.py, api-server.ts, openstax_content.py, deploy.py) but all have a default fallback of gemini-2.5.-flash, so it's optional.

The Quickstart notebook generates a minimal .env with the required environment variables (GOOGLE_CLOUD_PROJECT, AGENT_ENGINE_PROJECT_NUMBER, AGENT_ENGINE_RESOURCE_ID). I'm including samples/personalized-learning/.env.template as documentation for optional settings like GENAI_MODEL, access control (VITE_ALLOWED_DOMAIN/VITE_ALLOWED_EMAILS), and GCS buckets. Happy to remove if you feel it's redundant or confusing.

I confirmed that executing the demo works after having cleared .env and existing auths

@samgoodgame
Copy link
Contributor Author

Additional fixes (commits dbbcbc0, 0bce8e2) after testing end-to-end:

While running through the quickstart fresh, I hit issues from the Jan 23 web_core refactor (#512) that extracted shared code from @a2ui/lit into @a2ui/web_core:

Local dev — quickstart_setup.sh now builds web_core before lit
Cloud Run deployment — deploy_hosting.py now copies both web_core and lit to the build context and updates the package references
Local dev auth — Added server-side auth bypass in api-server.ts when VITE_FIREBASE_API_KEY isn't set (aligns with existing client-side behavior documented in firebase-auth.ts)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants